-
Notifications
You must be signed in to change notification settings - Fork 107
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove await from current minimal proposal spec #108
Conversation
Promises can resolve into functions. We’re changing the original/minimal proposal such that |
Parenthesizing it should work fine, provided that the promise resolves to a function. |
@ljharb: But without parentheses, it would be an error, under the current plan (babel/proposals#29 (comment), TC39 presentation), right? If so, then a supplemental grammar (covering unparenthesized |
We need to be very careful about requiring parenthesis for the purpose of disambiguating semantics (as opposed to merely disambiguating precedence). It might be a smell. |
@zenparsing: It is a smell, but it’s a fundamental part of how Proposal 1 currently addresses See also the bottom paragraphs of @tabatkins’ comment at tc39/proposal-smart-pipelines#3 (comment) and also tc39/proposal-smart-pipelines#3 (comment). There may be ways to ameliorate this smell for Proposal 1, as we keep developing all the options. We’re doing it this way for now to compare everything. |
Just to be clear, this isn't really intended as the final semantics of the pipeline proposal, but we can be certain that this behavior is not final. This makes |
@mAAdhaTTah: Making sure sure: Only unparenthesized We still need to add that supplemental grammar and early error to this pull request. If you are uncertain how to do that, ping me on the Babel slack; or I could try to take care of it later, after I handle some stuff with smart pipelines’ explainer/spec and the presentation. Thanks again for handling this. |
To provide some additional context, here is the conversation where @littledan made this suggestion. I'm thinking about the "minimal proposal" as a strawman we can use to compare the tradeoffs of the various proposals. @js-choi That's the intention, yes. |
I think that we should consider making That is, for the production |PipelineExpression| : |LogicalORExpression|, “It is an early Syntax Error if |LogicalORExpression| Contains This would make a supplemental covering production unnecessary. It would also prevent the inconsistency of |
8127eb7
to
5e936b4
Compare
@js-choi I updated to include the early error verbage. |
@mAAdhaTTah: Great, though you’ll want to use the Contains static semantic rule, rather than “appears”, and it should go in its own Early Errors subsection. Though this is probably good enough for now for a sketch, so we’ll deal with that later. |
If we wanted to be literally subset of the intersection both proposals, we would have to prohibit several other constructs as well. Given that we are not doing that, I don't see why to prohibit any more await cases. |
spec.html
Outdated
@@ -35,10 +35,10 @@ <h1>Syntax</h1> | |||
PipelineExpression[In, Yield, Await] : | |||
LogicalORExpression[?In, ?Yield, ?Await] | |||
[~Await] PipelineExpression[?In, ?Yield, ?Await] `|>` LogicalORExpression[?In, ?Yield, ?Await] | |||
[+Await] PipelineExpression[?In, ?Yield, ?Await] `|>` [lookahead <! {`await`}] LogicalORExpression[?In, ?Yield, ?Await] | |||
[+Await] PipelineExpression[?In, ?Yield, ?Await] `|>` `await` LogicalORExpression[?In, ?Yield, ?Await] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deleting the second line looks good, but deleting the first line makes pipeline not work in async functions at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll re-add it, but could you elaborate as to why that's the case? I think I'm misunderstanding the purpose of the lookahead here.
spec.html
Outdated
</ins> | ||
</emu-grammar> | ||
|
||
It is a Syntax Error if `await` appears in the `LogicalORExpression` the RHS of a `PipelineExpression`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you keep the lookahead restriction, it would ban literal await following a pipeline. With that, I don't see why we'd need a syntax error.
Nit: syntax errors tend to be presented in Static Semantics: Early Error blocks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is kinda what I thought but I wasn't sure. I'll remove this, since the wording isn't quite right.
This will now result in an early error when using `|> await f`. Updates the README as well.
I just removed what enabled it. I think this is all thats needed on the spec side, unless there should be some explicit indication of a
SyntaxError
here?